-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parallelize the provision
step
#2468
Conversation
3c87330
to
72c73fd
Compare
Hm, I've tried with virtual and it doesn't work well.
There should be 3 machines running but virsh shows that one of them is paused. |
Worked like a charm on my laptop :/ Can you check whether there's some libvirt/qemu/testcloud logging related to this? I'm not versed in Qemu things, but it looks like some kind of conflict. @frantisekz any idea what might be we hitting? |
72c73fd
to
94ec544
Compare
@happz I'd say https://pagure.io/testcloud/blob/master/f/testcloud/util.py#_60 is not ready for parallel requests. |
I'll take a look early December (currently on PTO without laptop). |
Right, that might be the case, easily.
Never mind, added it as a commit to this PR. |
I tried the latest patch with libvirt-provision plugin, it works. I got three guests provisioned parallelized. My plan: execute: /plan: log: |
Glad to hear that, thanks!
On another note: please, consider supporting hardware:
cpu:
cores: 2
memory: 4096 MB I know it's probably of no immediate benefit to you now, and I would fully understand the hesitation or focus on more needed features for your use cases, but in the long run, it would help end-users if plugins would speak the same language. The field is supported by tmt's own plugins, by Testing Farm, and we're slowly extending the requirements supported by individual plugins. I suppose your plugin is very similar to |
Hi @happz , Thanks for the suggestion of using TMT hardware specification and sharing the Thanks, |
I've tried the parallel provision with the
Twice it was just a single box which failed and once even two of them. @happz, have you seen anything like this? |
Nope, I haven't seen this error. Well, I'm afraid we should push this one out into 1.31. The PR is changing the whole step, you're hitting issues I haven't seen, there's barely any review, and it's Nov 28th :/ |
db099f3
to
f554c5a
Compare
/packit test --identifier full |
f554c5a
to
e13018a
Compare
ac12208
to
0b21eb3
Compare
Interesting. Tried today again and encountered similar behaviour. Seems that the problem happens randomly. Run four times:
Looking through the log I see:
Which is weird because the command line is the same as for the other guests:
Any idea what might be wrong? |
Stinks like a race condition, threads contesting shared resources or something similar. I'll try to debug it in the afternoon. |
Thanks. The detailed debug output from
Seems like the provided key is not accepted. |
0b21eb3
to
87ec95f
Compare
@psss hopefully addressed in 87ec95f: added more locking in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks much for implementing this! Looks good and works nicely. Added just some minor questions and suggestions.
This definitely deserves a mention in the release notes. Also, what do you think about adding a short paragraph with the overview of steps which support parallel execution to the Guide? |
87ec95f
to
db0510d
Compare
Sure, added one in a8d369a.
I mentioned compatible plugins in the release note, and I was hoping to make this information available on plugin pages, once #2549 lands |
Makes sense, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing all issues. Looks good now. I've just slightly modified the release note formatting in 59681ab.
59681ab
to
5a07401
Compare
5a07401
to
0aa7db6
Compare
Many tests are failing because of:
|
/packit test --identifier full
Adding b6a3250, let's see whether it helps. |
/packit test --identifier full |
This change requires refactoring of current queue internals, to better support the third kind of parallelization & queueing. A custom queue task is needed, to run provisioning in parallel, and the step takes care of ordering of provision phases and actions.
4a5e340
to
8196422
Compare
/packit test --identifier full |
Seems beaker provisioning is still in sequence, any reasons? Thanks. |
@@ -680,6 +680,8 @@ class ProvisionBeaker(tmt.steps.provision.ProvisionPlugin[ProvisionBeakerData]): | |||
_data_class = ProvisionBeakerData | |||
_guest_class = GuestBeaker | |||
|
|||
# _thread_safe = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uncomment it, use _thread_safe = True, beaker parallel provisioning works for me. Wondering why you comment it?
[client-1] guest: has been requested
[client-1] job id: 8784929
[client-1] wait: waiting for condition 'get_new_state' with timeout 1:00:00, deadline in 3600.0 seconds, checking every 60.00 seconds
[server-1] guest: has been requested
[server-1] job id: 8784930
[server-1] wait: waiting for condition 'get_new_state' with timeout 1:00:00, deadline in 3600.0 seconds, checking every 60.00 seconds
[client-2] guest: has been requested
[client-2] job id: 8784931
...
summary: 3 guests provisioned
client-2
client-1
server-1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we are not sure whether the library tmt uses for Beaker provisioning, mrack, is thread-safe. We did run into issues with the virtual
plugin, where the problem was eventually solved by adding an extra lock to provide serialization, but there was no time invested into doing the same for the Beaker plugin. If it's required, that is.
Filed #2607 to track this.
This change requires refactoring of current queue internals, to better support the third kind of parallelization & queueing. A custom queue task is needed, to run provisioning in parallel, and the step takes care of ordering of provision phases and actions.
Implements #2244
Pull Request Checklist